Placing allocs counts towards placement limit #3070
Conversation
This PR makes placing new allocations count towards the limit. We do not restrict how many new placements are made by the limit but we still count towards the limit. This has the nice affect that if you have a group with count = 5 and max_parallel = 1 but only 3 allocs exist for it and a change is made, you will create 2 more at the new version but not destroy one, taking you down to two running as you would have previously. Fixes #3053
| alloc.JobID = job.ID | ||
| alloc.NodeID = structs.GenerateUUID() | ||
| alloc.Name = structs.AllocName(job.ID, job.TaskGroups[0].Name, uint(i)) | ||
| alloc.TaskGroup = job.TaskGroups[0].Name |
There was a problem hiding this comment.
It looks like this happens in a couple other tests (assigning field values)- you could create a test helper function for this.
| @@ -374,6 +374,9 @@ func (a *allocReconciler) computeGroup(group string, all allocSet) bool { | |||
| for _, p := range place { | |||
| a.result.place = append(a.result.place, p) | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
Maybe not as part of this PR, but it would be good to limit the length of this function, and start introducing smaller functions to encompass some of this logic.
There was a problem hiding this comment.
Haha yeah it definitely needs a refactoring!
|
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
This PR makes placing new allocations count towards the limit. We do not
restrict how many new placements are made by the limit but we still
count towards the limit. This has the nice affect that if you have a
group with count = 5 and max_parallel = 1 but only 3 allocs exist for it
and a change is made, you will create 2 more at the new version but not
destroy one, taking you down to two running as you would have
previously.
Fixes #3053